Use diagnostic or language items instead of paths #6823

Jarcho · 2021-03-02T04:06:47Z

I think that gets everything except ones used in a list of paths to check.

changelog: none

rust-highfive · 2021-03-02T04:06:49Z

r? @phansch

(rust-highfive has picked a reviewer for you, use r? to override)

clippy_lints/src/lifetimes.rs

clippy_lints/src/types.rs

phansch · 2021-03-02T15:41:07Z

Minus the comments from @camsteffen, this looks good to me, thanks!

phansch · 2021-03-06T13:40:21Z

woops, misclicked 😅

phansch · 2021-03-06T13:40:30Z

@bors r+ thanks!

bors · 2021-03-06T13:40:31Z

📌 Commit 11375b5 has been approved by phansch

bors · 2021-03-06T13:46:41Z

⌛ Testing commit 11375b5 with merge f01d655...

Use diagnostic or language items instead of paths I think that gets everything except ones used in a list of paths to check. changelog: none

bors · 2021-03-06T13:57:33Z

💔 Test failed - checks-action_test

phansch · 2021-03-07T12:04:38Z

@bors r+

bors · 2021-03-07T12:04:39Z

📌 Commit e4ffff9 has been approved by phansch

bors · 2021-03-07T12:04:45Z

⌛ Testing commit e4ffff9 with merge 5945e85...

bors · 2021-03-07T12:21:39Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: phansch
Pushing 5945e85 to master...

matthiaskrgr · 2021-03-08T15:21:50Z

Is this expected to have a direct effect on lint results?
I'm asking because the change causes ~160 expl_impl_clone_on_copy warnings to appear in the lintcheck logs.

Most of them seem to be caused by macro usage

target/lintcheck/sources/libc-0.2.81/src/macros.rs:84:9

#[allow(unused_macros)]
macro_rules! s {
    ($($(#[$attr:meta])* pub $t:ident $i:ident { $($field:tt)* })*) => ($(
        s!(it: $(#[$attr])* pub $t $i { $($field)* });
    )*);
    (it: $(#[$attr:meta])* pub union $i:ident { $($field:tt)* }) => (
        compile_error!("unions cannot derive extra traits, use s_no_extra_traits instead");
    );
    (it: $(#[$attr:meta])* pub struct $i:ident { $($field:tt)* }) => (
        __item! {
            #[repr(C)]
            #[cfg_attr(feature = "extra_traits", derive(Debug, Eq, Hash, PartialEq))]
            #[allow(deprecated)]
            $(#[$attr])*
            pub struct $i { $($field)* }
        }
        #[allow(deprecated)]
        impl ::Copy for $i {}
        #[allow(deprecated)]
        impl ::Clone for $i { // <- HERE
            fn clone(&self) -> $i { *self }
        }
    );
}

warning: you are implementing `Clone` explicitly on a `Copy` type
    --> src/macros.rs:84:9
     |
84   | /         impl ::Clone for $i {
85   | |             fn clone(&self) -> $i { *self }
86   | |         }
     | |_________^
     |
    ::: src/unix/linux_like/linux/mod.rs:3502:1
     |
3502 |   expand_align!();
     |   ---------------- in this macro invocation
     |
note: consider deriving `Clone` or removing `Copy`
    --> src/macros.rs:84:9
     |
84   | /         impl ::Clone for $i {
85   | |             fn clone(&self) -> $i { *self }
86   | |         }
     | |_________^
     |
    ::: src/unix/linux_like/linux/mod.rs:3502:1
     |
3502 |   expand_align!();
     |   ---------------- in this macro invocation
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy
     = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

warning: you are implementing `Clone` explicitly on a `Copy` type
    --> src/macros.rs:120:9
     |
120  | /         impl ::Clone for $i {
121  | |             fn clone(&self) -> $i { *self }
122  | |         }
     | |_________^
     |
    ::: src/unix/linux_like/linux/mod.rs:3502:1
     |
3502 |   expand_align!();
     |   ---------------- in this macro invocation
     |
note: consider deriving `Clone` or removing `Copy`
    --> src/macros.rs:120:9
     |
120  | /         impl ::Clone for $i {
121  | |             fn clone(&self) -> $i { *self }
122  | |         }
     | |_________^
     |
    ::: src/unix/linux_like/linux/mod.rs:3502:1
     |
3502 |   expand_align!();
     |   ---------------- in this macro invocation
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy
     = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

warning: you are implementing `Clone` explicitly on a `Copy` type
    --> src/macros.rs:120:9
     |
120  | /         impl ::Clone for $i {
121  | |             fn clone(&self) -> $i { *self }
122  | |         }
     | |_________^
     |
    ::: src/unix/linux_like/linux/mod.rs:3502:1
     |
3502 |   expand_align!();
     |   ---------------- in this macro invocation
     |
note: consider deriving `Clone` or removing `Copy`
    --> src/macros.rs:120:9
     |
120  | /         impl ::Clone for $i {
121  | |             fn clone(&self) -> $i { *self }
122  | |         }
     | |_________^
     |
    ::: src/unix/linux_like/linux/mod.rs:3502:1
     |
3502 |   expand_align!();
     |   ---------------- in this macro invocation
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy
     = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

warning: you are implementing `Clone` explicitly on a `Copy` type
    --> src/macros.rs:120:9
     |
120  | /         impl ::Clone for $i {
121  | |             fn clone(&self) -> $i { *self }
122  | |         }
     | |_________^
     |
    ::: src/unix/linux_like/linux/mod.rs:3502:1
     |
3502 |   expand_align!();
     |   ---------------- in this macro invocation
     |
note: consider deriving `Clone` or removing `Copy`
    --> src/macros.rs:120:9
     |
120  | /         impl ::Clone for $i {
121  | |             fn clone(&self) -> $i { *self }
122  | |         }
     | |_________^
     |
    ::: src/unix/linux_like/linux/mod.rs:3502:1
     |
3502 |   expand_align!();
     |   ---------------- in this macro invocation
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy
     = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

flip1995 · 2021-03-08T15:31:00Z

clippy_lints/src/derive.rs

@@ -293,7 +293,7 @@ fn check_ord_partial_ord<'tcx>(

 /// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint.
 fn check_copy_clone<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &TraitRef<'_>, ty: Ty<'tcx>) {
-    if match_path(&trait_ref.path, &paths::CLONE_TRAIT) {
+    if cx.tcx.lang_items().clone_trait() == trait_ref.trait_def_id() {


Maybe comparing def_ids isn't equivalent to the match_path method?

The lang item might not be defined at the point. I didn't think that would actually happen though. I'll have a pr in a bit.

Jarcho · 2021-03-09T02:32:41Z

Turns out this did fix expl_impl_clone_on_copy, which is causing all those warnings. #6868 (comment)

flip1995 · 2021-03-09T09:23:10Z

Yeah, I agree. Triggering in the above example makes sense to me. I also think triggering in local macros is fine.

Don't assume lang items will exist. ~~Should fix lintcheck warnings caused by #6823~~ See below changelog: None

rust-highfive assigned phansch Mar 2, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 2, 2021

Use diagnostic or language items instead of paths

f21320f

Jarcho force-pushed the diagnostic_items branch from b9b1620 to f21320f Compare March 2, 2021 04:11

camsteffen suggested changes Mar 2, 2021

View reviewed changes

clippy_lints/src/lifetimes.rs Outdated Show resolved Hide resolved

clippy_lints/src/types.rs Outdated Show resolved Hide resolved

phansch closed this Mar 6, 2021

phansch reopened this Mar 6, 2021

bors added a commit that referenced this pull request Mar 6, 2021

Auto merge of #6823 - Jarcho:diagnostic_items, r=phansch

f01d655

Use diagnostic or language items instead of paths I think that gets everything except ones used in a list of paths to check. changelog: none

Use LanguageItems::require

e4ffff9

Jarcho force-pushed the diagnostic_items branch from 11375b5 to e4ffff9 Compare March 6, 2021 18:03

bors merged commit 5945e85 into rust-lang:master Mar 7, 2021

flip1995 reviewed Mar 8, 2021

View reviewed changes

Jarcho mentioned this pull request Mar 8, 2021

Don't assume lang items will exist. #6868

Merged

bors added a commit that referenced this pull request Mar 9, 2021

Auto merge of #6868 - Jarcho:lang_item, r=flip1995

627e910

Don't assume lang items will exist. ~~Should fix lintcheck warnings caused by #6823~~ See below changelog: None

Jarcho deleted the diagnostic_items branch April 6, 2021 16:09

Use diagnostic or language items instead of paths #6823

Use diagnostic or language items instead of paths #6823

Uh oh!

Conversation

Jarcho commented Mar 2, 2021

Uh oh!

rust-highfive commented Mar 2, 2021

Uh oh!

Uh oh!

Uh oh!

phansch commented Mar 2, 2021

Uh oh!

phansch commented Mar 6, 2021

Uh oh!

phansch commented Mar 6, 2021

Uh oh!

bors commented Mar 6, 2021

Uh oh!

bors commented Mar 6, 2021

Uh oh!

bors commented Mar 6, 2021

Uh oh!

phansch commented Mar 7, 2021

Uh oh!

bors commented Mar 7, 2021

Uh oh!

bors commented Mar 7, 2021

Uh oh!

bors commented Mar 7, 2021

Uh oh!

matthiaskrgr commented Mar 8, 2021

Uh oh!

flip1995 Mar 8, 2021

Choose a reason for hiding this comment

Uh oh!

Jarcho Mar 8, 2021

Choose a reason for hiding this comment

Uh oh!

Jarcho commented Mar 9, 2021

Uh oh!

flip1995 commented Mar 9, 2021

Uh oh!

Uh oh!